Repository navigation
feat(desktop): restore Workspaces tab with host-scoped workspace list - #5908
Conversation
Bring back the sidebar Workspaces entry (removes the SHOW_WORKSPACES_TAB flag) and rework /v2-workspaces to read from a single explicitly-targeted host instead of the all-hosts fan-out: - New useSelectedHostWorkspaces: one workspace.list against the selected host, live-patched by its workspace:changed events, IndexedDB snapshot for offline, no Electric v2Workspaces fallback. - Device filter defaults to "This device"; the all-devices merged view is removed. Host picker is populated instantly from the v2Hosts and v2UsersHosts collections (no per-host counts - counting is a fan-out). - Host-served rows no longer require v2Hosts/v2UsersHosts rows to render; the cloud-host-table gate applies only to cloud-fallback rows. - PR enrichment runs only for the host-scoped page, not for unscoped callers (command palette, dev sidebar seeding). - Empty state is gated on the selected host's settlement so an unanswered host reads as loading, not empty. Claude-Session: https://claude.ai/code/session_01ULuSGAMv3cqtqBMjaYzDqd
📝 WalkthroughWalkthroughThe Workspaces sidebar entry is restored unconditionally. V2 workspaces now default to the current device, remove device counts, support scoped host loading, expose readiness state, derive host options from cached hosts, and gate empty-state rendering until data is ready. ChangesWorkspace navigation and host-scoped data flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant V2WorkspacesPage
participant useAccessibleV2Workspaces
participant useHostWorkspacesSource
participant HostWorkspaces
V2WorkspacesPage->>useAccessibleV2Workspaces: request filtered workspace data
useAccessibleV2Workspaces->>useHostWorkspacesSource: provide selected host id
useHostWorkspacesSource->>HostWorkspaces: query matching host targets
HostWorkspaces-->>useHostWorkspacesSource: workspace rows and query state
useHostWorkspacesSource-->>useAccessibleV2Workspaces: scoped rows and isReady
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Delete useSelectedHostWorkspaces - it duplicated the source hook's target derivation, snapshots, query config, and event patching. The source hook now takes an optional scopedHostId (undefined = fan-out, machine id = single host with no cloud fallback, null = scoped but unresolved, runs nothing). Also drop the now-dead device-filter stage (scoped rows are already single-host) and the duplicated empty-state wrapper. Claude-Session: https://claude.ai/code/session_01ULuSGAMv3cqtqBMjaYzDqd
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts`:
- Around line 68-70: The per-host snapshot hydration and workspace:changed
listener ownership currently runs once per useHostWorkspacesSource instance,
causing duplicate persistence and event patching for scoped and unscoped
consumers. In useHostWorkspacesSource, move or share this lifecycle work through
provider-backed ownership so each host has only one hydrator and retained
listener; update the guidance in HostWorkspacesProvider to clarify that scoped
calls are not inherently safe for non-query lifecycle work. Affected sites:
apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts
lines 68-70 require the lifecycle deduplication;
apps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider/HostWorkspacesProvider.tsx
lines 14-15 require the guidance update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cca1312a-ce40-4ce5-b916-1cb1cc98d749
📒 Files selected for processing (4)
apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAccessibleV2Workspaces/useAccessibleV2Workspaces.tsapps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider/HostWorkspacesProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAccessibleV2Workspaces/useAccessibleV2Workspaces.ts
| export function useHostWorkspacesSource( | ||
| scopedHostId?: string | null, | ||
| ): UseHostWorkspacesResult { |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm simultaneous unscoped/provider and scoped/page source-hook usage.
rg -n -C3 --type ts 'useHostWorkspacesSource\s*\(' apps/desktop/src/renderer
# Inspect the source-local listener and retain lifecycle.
rg -n -C5 --type ts '"workspace:changed"|\.retain\(\)|getEventBus' \
apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.tsRepository: superset-sh/superset
Length of output: 4436
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useHostWorkspaces outline =="
ast-grep outline apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts --view expanded || true
echo "== source-hook key/runtime sections =="
cat -n apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts | sed -n '1,260p'
echo "== HostWorkspacesProvider =="
cat -n apps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider/HostWorkspacesProvider.tsx | sed -n '1,80p'
echo "== v2Workspaces relevant source =="
cat -n apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAccessibleV2Workspaces/useAccessibleV2Workspaces.ts | sed -n '220,260p'
echo "== query key implementation =="
rg -n -C4 --type ts 'function getHostWorkspacesQueryKey|const getHostWorkspacesQueryKey|`@tanstack/react-query`|useQueries|useQueryClient|useLiveQuery|retain\(\)' apps/desktop/src/renderer/hooks/host-workspaces apps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAccessibleV2WorkspacesRepository: superset-sh/superset
Length of output: 38368
Deduplicate scoped source per-host lifecycle work.
HostWorkspacesProvider mounts an unscoped useHostWorkspacesSource() while useAccessibleV2Workspaces also mounts useHostWorkspacesSource(selectedHostId) for the selected host. Shared query keys coalesce results, but each hook instance hydrates the IndexedDB snapshot and registers its own workspace:changed listener with bus.retain(), so selected-host events are patched and persisted twice.
apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts#L68-L70: move per-host snapshot/event ownership into a shared provider-backed source, or otherwise share it across scoped and unscoped consumers.apps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider/HostWorkspacesProvider.tsx#L14-L15: update the guidance to avoid implying scoped calls are fully safe for non-query lifecycle work.
📍 Affects 2 files
apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts#L68-L70(this comment)apps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider/HostWorkspacesProvider.tsx#L14-L15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts`
around lines 68 - 70, The per-host snapshot hydration and workspace:changed
listener ownership currently runs once per useHostWorkspacesSource instance,
causing duplicate persistence and event patching for scoped and unscoped
consumers. In useHostWorkspacesSource, move or share this lifecycle work through
provider-backed ownership so each host has only one hydrator and retained
listener; update the guidance in HostWorkspacesProvider to clarify that scoped
calls are not inherently safe for non-query lifecycle work. Affected sites:
apps/desktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.ts
lines 68-70 require the lifecycle deduplication;
apps/desktop/src/renderer/routes/_authenticated/providers/HostWorkspacesProvider/HostWorkspacesProvider.tsx
lines 14-15 require the guidance update.
pinned/others/V2WorkspaceProjectGroup had no consumers - delete them. Collapse the scoped-vs-fanout selection into one destructure with a comment explaining why both hooks always run, and document that a null scopedHostId runs nothing. Claude-Session: https://claude.ai/code/session_01ULuSGAMv3cqtqBMjaYzDqd
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Main (#5908) replaced counts with isReady in useAccessibleV2Workspaces — textually clean merge but semantically broken (the pinned empty-state check referenced the removed counts). Resolved by gating the pinned create screen on isReady + zero rows + default filters, which also fixes the transient-zero flash the old counts check was exposed to. Claude-Session: https://claude.ai/code/session_01SfU6AHAcHwrjwXwUFUDth5
Summary
Brings the Workspaces sidebar tab back for real (no flag, no setting) and reworks the
/v2-workspacespage to be host-first: the device picker loads instantly from the hosts collections, and the workspace list always reads from exactly one explicitly-targeted host — no fan-out, no reliance on the cloudv2_workspacestable being up to date.Closes #5881. Supersedes #5901 (which kept the tab behind an off-by-default Appearance toggle).
Changes
Tab restored —
SHOW_WORKSPACES_TABdeleted fromDashboardSidebarHeader; both the collapsed-rail icon and the expanded row render unconditionally.Single-host reads —
useHostWorkspacesSourcenow takes an optionalscopedHostId: the page passes the selected host and gets oneworkspace.list(local direct, remote via relay), live-patched by that host'sworkspace:changedevents, with the per-host IndexedDB snapshot for offline and deliberately no Electricv2Workspacesfallback — an unreachable host shows its last-seen snapshot or nothing, never stale cloud rows. No new hook: scoped calls reuse the fan-out's target derivation, query keys, snapshots, and event patching, so where the provider is mounted the host is fetched once, not twice.Always host-scoped — the "All devices" merged view is removed; the device filter defaults to This device. Ten idle/slow hosts can no longer delay the page or silently thin out the list: readiness and failure are per selected host.
Instant host picker — options come from
v2Hosts∩v2UsersHosts(locally cached Electric data), so every accessible host appears immediately, including hosts with zero workspaces. Per-host workspace counts are dropped — counting other hosts' workspaces would itself be a fan-out.Host-served rows are their own proof — the join previously required a
v2Hostsrow and av2UsersHostsrow before rendering a workspace, so an empty/stale cloud host table hid live host data (caught during CDP verification below). That gate now applies only to cloud-fallback rows; host name/online status degrade gracefully when the cloud row is missing.Fan-out trims — PR-status enrichment (
pullRequests.getByWorkspaces, 10s interval) now runs only for the host-scoped page; unscoped callers (command palette search, dev sidebar seeding) don't render PR data and previously triggered it per host. Those callers keep the shared provider's merged view via the hook's no-deviceFilterpath.Empty vs loading — the empty state is gated on the selected host's settlement, so an unanswered host reads as loading, not "No workspaces yet".
Out of scope: the app-wide
HostWorkspacesProviderfan-out still exists — the dashboard sidebar and palette genuinely need cross-host data. Taking those off the fan-out is its own ticket.Testing
bun run lintandbun run --cwd apps/desktop typecheckclean.#/workspace/<id>→#/v2-workspaces; page rendered 27 workspaces host-scoped to "This device" with the picker defaulting correctly; navigate-away/back and a full reload both re-rendered the populated list. Screenshots captured at each step.v2Hostswas empty in that environment — fixed (host-served rows bypass the cloud gate) and re-verified populated after remount and reload.https://claude.ai/code/session_01ULuSGAMv3cqtqBMjaYzDqd
Summary by cubic
Restores the Workspaces tab and makes the Workspaces page host-scoped for faster loads and clearer states. Folds single-host reads into
useHostWorkspacesSourceand simplifies filters and outputs.New Features
useHostWorkspacesSource(scopedHostId?): singleworkspace.list, liveworkspace:changedupdates, IndexedDB snapshot for offline; no cloudv2Workspacesfallback when scoped.v2Hosts/v2UsersHosts(no per-host counts).Refactors
undefinedfans out once in the provider; a host id targets one host;nulldefers and runs nothing. Scoped calls share query keys; avoid unscoped duplication outside the provider.useAccessibleV2Workspaces: removedpinned/othersand device counts; now returnsisReadyto gate empty states (list waits for the selected host to settle).Written for commit 227f853. Summary will update on new commits.
Summary by CodeRabbit